Skip to content
This repository was archived by the owner on Mar 27, 2025. It is now read-only.

Conversation

@xry111
Copy link
Contributor

@xry111 xry111 commented Jul 25, 2022

In the BFD code (under review now), we have:

#define RELOCATE_CALC_PC32_HI20(relocation, pc) 	\
  ({						\
    bfd_vma lo = (relocation) & ((bfd_vma)0xfff);	\
    pc = pc & (~(bfd_vma)0xfff);			\
    if (lo > 0x7ff)					\
      {						\
        relocation += 0x1000;			\
      } 						\
    relocation &= ~(bfd_vma)0xfff;			\
    relocation -= pc;				\
  })

#define RELOCATE_CALC_PC64_HI32(relocation, pc)  	\
  ({						\
    bfd_vma lo = (relocation) & ((bfd_vma)0xfff);	\
    if (lo > 0x7ff)					\
      { 						\
        relocation -= 0x100000000;      		\
      }  						\
    relocation -= (pc & ~(bfd_vma)0xffffffff);  	\
  })

Pay attention at relocation += 0x1000 and relocation -= 0x100000000.

Currently our detailed description does not strictly match it.
This will puzzle the engineer implementing or reviewing the relocs for
other linkers (gold/lld/mold) and cause we criticized by the upstream
reviewers. Fix the pseudo-code in the "detail" column and add two
footnotes as explanation about why the additional operation is needed.

In the BFD code (under review now), we have:

    #define RELOCATE_CALC_PC32_HI20(relocation, pc) 	\
      ({						\
        bfd_vma lo = (relocation) & ((bfd_vma)0xfff);	\
        pc = pc & (~(bfd_vma)0xfff);			\
        if (lo > 0x7ff)					\
          {						\
            relocation += 0x1000;			\
          } 						\
        relocation &= ~(bfd_vma)0xfff;			\
        relocation -= pc;				\
      })

    #define RELOCATE_CALC_PC64_HI32(relocation, pc)  	\
      ({						\
        bfd_vma lo = (relocation) & ((bfd_vma)0xfff);	\
        if (lo > 0x7ff)					\
          { 						\
            relocation -= 0x100000000;      		\
          }  						\
        relocation -= (pc & ~(bfd_vma)0xffffffff);  	\
      })

Pay attention at `relocation += 0x1000` and `relocation -= 0x100000000`.

Currently our detailed description does not strictly match it.
This will puzzle the engineer implementing or reviewing the relocs for
other linkers (gold/lld/mold) and cause we criticized by the upstream
reviewers.  Fix the pseudo-code in the "detail" column and add two
footnotes as explanation about why the additional operation is needed.
Copy link
Contributor

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

认同。我自己实现 LLD 时候很是烧了一些脑细胞。。

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants